Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pin bottle version to fix CI runs #541

Closed

Conversation

glatterf42
Copy link
Member

@glatterf42 glatterf42 commented Sep 9, 2024

Four days ago, bottle released version 0.13.0 and v0.13.1 two days afterwards. Unfortunately, these versions are incompatible with pretenders, which we use in test_access to mock a local http server which we can target with access-related requests.

Pretenders hasn't been updated in two years, so I think it's unlikely we'll receive a patch here. For clarification, I've still opened pretenders/pretenders#153, though.

To replace pretenders, I thought about using monkeypatch to patch the result of self._backend.get_auth() in Platform.check_access(), but that would probably miss the point since we want to test this authentication-getting function, I think.
I've considered responses (as suggested here) but couldn't quite get it to work. Maybe the information I'm missing is not in their docs or maybe it's just too awkward to configure a mock server response for the authentication code, which seems to be baked into the old java code.
Next, I've tried pytest-httpserver, which just needs to be installed with pip and is then immediately available as a pytest fixture. I got a little further here, did at least manage to get the login request sent to the mock server, but still couldn't make it work because I was apparently missing something about json-deserialization, strings and lists in java:

Caused by: com.fasterxml.jackson.databind.exc.MismatchedInputException: Cannot deserialize instance of `java.util.ArrayList` out of VALUE_STRING token

Lastly, I considered migrating the code from pretenders into our code base (the essential part at least) and using bottle directly. However, this seems unwise in terms of maintenance. So for now, this PR pins the bottle version to the latest working one. In an ideal world, we put a bit more focus on developing ixmp4 and begin the migration still this year, so that we don't have to worry about this for too long.
Alternatively, we might have to revisit this and fix our mocking approach when the pinned bottle version doesn't support the latest python version or some such.

How to review

Please jump in here if you have a good idea for a sustainable solution. Otherwise, just note the CI checks all pass ... for now.

@khaeru Please let me know if we should also put this pin in pyproject.toml. If we don't, new installations will probably end up with not-working dependencies.

PR checklist

  • Continuous integration checks all ✅
  • [ ] Add or expand tests; coverage checks both ✅ Just CI.
  • [ ] Add, expand, or update documentation. Just CI.
  • Update release notes.

@glatterf42 glatterf42 added bug backend.jdbc Interaction with ixmp_source via JDBCBackend & JPype dependencies Pull requests that update a dependency file labels Sep 9, 2024
@glatterf42 glatterf42 self-assigned this Sep 9, 2024
Copy link

codecov bot commented Sep 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.8%. Comparing base (e12922f) to head (36297e2).
Report is 45 commits behind head on main.

Additional details and impacted files
@@          Coverage Diff          @@
##            main    #541   +/-   ##
=====================================
  Coverage   98.8%   98.8%           
=====================================
  Files         44      44           
  Lines       4813    4813           
=====================================
  Hits        4756    4756           
  Misses        57      57           

@glatterf42 glatterf42 force-pushed the pin/bottle-pretenders-breakage branch 2 times, most recently from 39729d1 to 36297e2 Compare September 9, 2024 15:13
@glatterf42
Copy link
Member Author

I have no idea what's going on with the windows CI runs. Apparently, our CI relies heavily on cached results being reused because if it stops once and need to pick up without the cache, all sorts of things come crashing down :(
In this case, the windows runners don't pick up the GAMS_LICENSE environment variable anymore. I tried adapting the setup-gams action, see here for my various attempts, but everything I tried made things worse despite seeming logical to me.
Maybe tomorrow I'll have a better idea.

@glatterf42
Copy link
Member Author

Superseded by #542.

@glatterf42 glatterf42 closed this Sep 10, 2024
@glatterf42 glatterf42 deleted the pin/bottle-pretenders-breakage branch September 10, 2024 06:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend.jdbc Interaction with ixmp_source via JDBCBackend & JPype bug dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant